Skip to content

Replace CStr::from_ptr() with CStr::from_bytes_with_nul() #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 4, 2025
Merged

Conversation

MarijnS95
Copy link
Member

For #81 (comment)

CStr::from_ptr() is unsafe because it reads a raw pointer, and searches for a terminating nul character in the pointed region of memory.

This is unnecessary as both calls already initialize a given number of characters and terminate with a nul, allowing us to pass a sized and initialized slice (without casting *const MaybeUninit<u8> to *const u8) directly to CStr::from_bytes_with_nul() (available since Rust 1.10, unlike CStr::from_bytes_until_nul() which was only stabilized in 1.69). Unfortunately all std helper APIs to initialize slices of MaybeUninit are still unstable, making this less ideal to write at the moment.

@Nercury
Copy link
Collaborator

Nercury commented Mar 4, 2025

Sorry for noticing your PR a bit late. Just ping me next time, it's ok. Let me know when you are ready for review.

@MarijnS95
Copy link
Member Author

MarijnS95 commented Mar 4, 2025

@Nercury nice timing; I was updating it as we speak after noticing I left this sit idle :). It's ready for review now.

@Nercury
Copy link
Collaborator

Nercury commented Mar 4, 2025

Whoopsie it has merge conflict now.

`CStr::from_ptr()` is `unsafe` because it reads a raw pointer, and
searches for a terminating nul character in the pointed region of
memory.

This is unnecessary as both calls already initialize a given number of
characters and terminate with a nul, allowing us to pass a sized and
initialized slice (without casting `*const MaybeUninit<u8>` to `*const
u8`) directly to `CStr::from_bytes_with_nul()` (available since Rust
1.10, unlike `CStr::from_bytes_until_nul()` which was only stabilized
in 1.69).  Unfortunately all `std` helper APIs to initialize slices of
`MaybeUninit` are still unstable, making this less ideal to write at
the moment.
@MarijnS95 MarijnS95 requested a review from Nercury March 4, 2025 14:16
@Nercury Nercury merged commit 401a08d into master Mar 4, 2025
24 checks passed
@MarijnS95 MarijnS95 deleted the cstr branch March 4, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants